Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refactor macro tests so that we can move macros #524

Merged
merged 3 commits into from
Dec 5, 2023

Conversation

benc-db
Copy link
Collaborator

@benc-db benc-db commented Dec 4, 2023

Description

This PR was originally going to be reorganizing the macros to be more aligned with dbt-core. When I tried to move a macro, I realized how insane our macro tests were, as they all broke.

In this PR:

  • Move create_table_as to be aligned with dbt-core organization
  • Refactor macro tests to use pytest fixtures
  • Refactor the base of the macro tests so that an inheriting test can specify which templates it needs to load into the context in order for the render to work properly.

Now, in a world of fine-grained unit tests, this would not be as gnarly, because part of the issue here is needing to load in the dependencies of the macro. For example, if your macro references file_format_clause(), you'll need the template context to know how to resolve that. In this case, it comes from dbt-spark, in the adapters.sql file. An alternative approach is to mock out everything, but it may be quite hard to judge if the generated sql is actually something that will work. So instead, I give pytest fixtures that can be overridden in order to give whatever template resolution you need to get the macros to work. In my next commit, I'll add documentation to base.

Checklist

  • I have run this code in development and it appears to resolve the stated issue
  • This PR includes tests, or tests are not required/relevant for this PR
  • I have updated the CHANGELOG.md and added information about my change to the "dbt-databricks next" section.

Copy link
Collaborator

@rcypher-databricks rcypher-databricks left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Lot's of really good work here. Thanks for doing this.

tests/unit/macros/base.py Outdated Show resolved Hide resolved
@benc-db benc-db merged commit 3326ec5 into main Dec 5, 2023
21 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants